Skip to content

perf(sync): overlap pre-unlock pull with the password prompt (#103, Tier 1) - #109

Merged
arimxyer merged 1 commit into
mainfrom
perf/103-concurrent-unlock
Jun 26, 2026
Merged

perf(sync): overlap pre-unlock pull with the password prompt (#103, Tier 1)#109
arimxyer merged 1 commit into
mainfrom
perf/103-concurrent-unlock

Conversation

@arimxyer

Copy link
Copy Markdown
Collaborator

Supersedes #108 (auto-closed when its base branch fix/102-content-hash-marker was deleted on #102's merge). #102 is now in main; this is rebased onto it and contains Tier-1 changes only.

What

Every sync-enabled read/write does a synchronous pre-unlock pull (CheckRemoteMetadatarclone lsjson, a network round-trip) before the vault unlocks. Tier 1 hides that latency for password-prompt users by running the pull concurrently with the master-password prompt — where the human is already waiting — with no staleness tradeoff (unlike a TTL).

The ordering constraint

Unlock reads + decrypts vault.enc in one step, and a pull replaces that file, so the decrypt must run strictly after the pull. Only work that doesn't touch vault.enc may overlap it.

  • New cmd/unlockVaultWithSync replaces the sequential syncPullBeforeUnlock(vs) + unlockVault(vs) pair at the 6 pull-path commands (add/delete/list/get/exec/update). usage/migrate, which never pulled, are unchanged.
  • Password path: kick off SyncPull in a goroutine → prompt + read the password (stdin only) → join the channel on every return path (so rclone is never orphaned on a prompt error/Ctrl-C) → decrypt against the now-current file.
  • Keychain path stays sequential: retrieving the keychain password never touches vault.enc, but there's no prompt to hide behind.

Scope: Tier 1 only — Tier 2 (the common case) is deferred to #103

Keychain is the default unlock and gets ~zero benefit here (no prompt to overlap). Tier 2 — overlapping the Argon2 KDF for keychain users — is the headline win and is intentionally split out (it's where the concurrency/crypto risk lives and needs its own -race treatment). Prerequisite verified: SaveVault/SaveVaultWithDEK only touch UpdatedAt, so salt, iterations, and wrappedDEK are invariant across normal saves → Tier 2 is viable with a post-join re-derive fallback. #103 stays open, scoped to Tier 2.

Tier 1 is still worth shipping alone: it's the goroutine+join skeleton Tier 2 builds on, and a real win for headless/CI/scripted runs where keychain is absent.

Correctness details

  • Conflict re-surfacing: SyncPull's conflict warning can print mid-prompt (garbled) and read commands never re-echo it (only writes do, via SyncPush). New vault.SyncConflictDetected() lets the orchestrator re-print the conflict cleanly below the prompt after the join, so the "sync resolve" signal is never lost.
  • RetrieveKeychainPassword: retrieves the master password from the keychain without decrypting (the seam enabling overlap). UnlockWithKeychain now calls it then Unlock — behavior unchanged.
  • Feedback: no transient indicator during the overlap (it would corrupt the prompt line); the verbose pull line is a full line above the prompt. TUI stays sequential (low-frequency, interactive) — a deliberate scope call.

Tests

RetrieveKeychainPassword (not-enabled → ErrKeychainNotEnabled); SyncConflictDetected default; and a -race test driving the concurrent password branch end-to-end against a real sync-enabled, initialized vault. Full unit + integration suites green; vault+cmd pass go test -race; golangci-lint v2.5 clean.

Refs #103 (Tier 1; Tier 2 remains tracked in #103).

🤖 Generated with Claude Code

…ier 1)

Every read/write that has sync enabled does a synchronous pre-unlock pull
(CheckRemoteMetadata → rclone lsjson, a network round-trip) before the vault
unlocks. Tier 1 hides that latency for password-prompt users by running the pull
concurrently with the master-password prompt — work where the human is already
waiting — with no staleness tradeoff.

Ordering constraint: Unlock reads + decrypts vault.enc in one step and a pull
replaces that file, so the decrypt must run strictly after the pull. Only work
that doesn't touch vault.enc may overlap:

- New cmd/unlockVaultWithSync replaces the sequential `syncPullBeforeUnlock(vs)`
  + `unlockVault(vs)` pair at the 6 pull-path commands (add/delete/list/get/
  exec/update). usage/migrate, which never pulled, are unchanged.
- Password path: kick off SyncPull in a goroutine, prompt + read the password
  (stdin only — no vault.enc access), JOIN on the channel (on every return path,
  so rclone is never orphaned), then decrypt against the now-current file.
- Keychain path stays sequential: retrieving the keychain password never touches
  vault.enc but there's no prompt to hide behind. Tier 2 (overlapping the Argon2
  KDF for keychain users — the common case) is a deferred follow-up; salt,
  iterations and wrappedDEK are all invariant across normal saves, so it's
  viable but carries the real concurrency/crypto risk and is split out.

Supporting changes in internal/vault:
- RetrieveKeychainPassword: retrieve the master password from the keychain
  WITHOUT decrypting (the seam that lets the pull overlap). UnlockWithKeychain
  now calls it then Unlock — behavior unchanged.
- SyncConflictDetected getter: SyncPull's conflict warning can print mid-prompt
  (garbled) and read commands never re-echo it (only writes do, via SyncPush) —
  so the orchestrator re-surfaces the conflict cleanly below the prompt after
  the join, ensuring the "sync resolve" signal is never lost.

Feedback: no transient indicator during the overlap (it would corrupt the prompt
line); the prompt itself signals work in flight, and the verbose pull line is a
full line printed above the prompt. TUI stays on the sequential path (low-
frequency, interactive) — a deliberate scope call.

Tests: RetrieveKeychainPassword (not-enabled → ErrKeychainNotEnabled);
SyncConflictDetected default; and a -race test driving the concurrent password
branch end-to-end against a real sync-enabled, initialized vault. Full unit +
integration suites green; vault+cmd pass `go test -race`.

Refs #103 (Tier 1; Tier 2 tracked separately).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019sxsM218vNzDbuMZ2nhMzx
@arimxyer
arimxyer merged commit 3c24a67 into main Jun 26, 2026
7 checks passed
@arimxyer
arimxyer deleted the perf/103-concurrent-unlock branch June 26, 2026 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant